Skip to content

Conversation

@lcaouen
Copy link
Contributor

@lcaouen lcaouen commented Nov 28, 2025

Shows in the About dialog box the incorrect settings from settings.ini.
It compares the settings from the settings.ini to the settings from the preferences.properties
The incorrect ones are displayed in red

This is related to this issue :
#3605

  • Testing:

  • Documentation:

    • A message is displayed at the top of the view in the about dialog saying the settings in red are incorrects

@georgweiss
Copy link
Collaborator

georgweiss commented Dec 1, 2025

Unfortunately not able to get the intended behavior.
Starting without any path to a settings file I get:

Screenshot 2025-12-01 at 09 16 20

Starting with a custom settings file (correct settings) specified through -settings /path/to/settings/file.ini I get:

Screenshot 2025-12-01 at 09 39 03

@lcaouen
Copy link
Contributor Author

lcaouen commented Dec 1, 2025

Don't know what I've done in my PR but some code is missing. I'm gonna check tomorrow

@lcaouen
Copy link
Contributor Author

lcaouen commented Dec 2, 2025

Even if there is some code missing, the code in this PR should work.
To find the correct settings, I use the classpath variable to open all the jars and then load all the settings.
Then I compare these settings to the ones in settings.ini.

I think your classpath is empty, because you use another method to load the jars.
We don't use the .sh or .bat provided during the build which apparently use a "main" jar to load all the other jars.
I 'm gonna check what I can do to take into account this other method, and also add the code I lost which was using another font and escaping the potential html in the settings

@lcaouen
Copy link
Contributor Author

lcaouen commented Dec 2, 2025

Here is the result I get when it works ;-)

image

@georgweiss
Copy link
Collaborator

Are you launching Phoebus with a -classpath command line argument?

@lcaouen
Copy link
Contributor Author

lcaouen commented Dec 2, 2025

Are you launching Phoebus with a -classpath command line argument?

Yes we put the lib path in the -classpath whereas the .sh or .bat to launch Phoebus with the official version uses the product-*.jar and the classpath in the manifest

@lcaouen
Copy link
Contributor Author

lcaouen commented Dec 2, 2025

I have updated the source code.
If there is only one jar in the classpath, it opens it to get the paths of the other jars.
I have also added the "misssing code"

  • a monospace font
  • escape the html tags in the properties
  • a message at the top saying properties in red are incorrects
image

@lcaouen
Copy link
Contributor Author

lcaouen commented Dec 11, 2025

@georgweiss is this new code ok ? Does it work for you ?

@georgweiss
Copy link
Collaborator

georgweiss commented Dec 11, 2025

@lcaouen, sorry for delayed feed-back.
Unfortunately I still see some issues with this.

When running in my IDE (IntelliJ) I get the following:

Screenshot 2025-12-11 at 10 05 11

When running Phoebus from a build I get the following:

Screenshot 2025-12-11 at 10 18 42

Here I find for instance org.csstudio.display.builder.editor.app.DisplayEditorInstance/snap_widgets=true, which is surprising as I do not have it in my settings file, nor can I find it in any preferences files in the project.
Also, org.phoebus.applications.alarms/server=localhost:9092 is a valid setting, so it should not be highlighted in red.

Moreover, settings like external_app_* cannot be evaluated to be valid or not as these have no entries in the properties files in the project. For instance, user may define a setting like org.phoebus.framework.workbench/external_app_json=JSON Viewer,json,code:

Screenshot 2025-12-11 at 10 29 35

@lcaouen
Copy link
Contributor Author

lcaouen commented Dec 11, 2025

No problem, thanks for your feedback.

I get the same problem with intellij on my computer, I'm gonna check what's the problem.
Concerning your build, would it be possible to download it from some website, in order to check what can be wrong ?
I'm also interested in your settings.ini (remove any value which can be confidential).
As you can see in my previous screenshot I don't have the problem with the key org.phoebus.applications.alarms/server=
So it would help me to find the problem.

I'm surprised by the fact you get an extra key as I think I haven't modify the part of the code which loads the settings.ini.
Could you check if the org.csstudio.display.builder.editor.app.DisplayEditorInstance/snap_widgets is present or not with a build from the master branch ?

Concerning org.phoebus.framework.workbench/external_app_json, if there is no .properties, what do you think about ignoring key with /external_app in the name ?

@georgweiss
Copy link
Collaborator

@lcaouen, I have uploaded builds to https://drive.google.com/drive/folders/10ERyQAEioZCjrJpj2OVljKSzTI3e7PfO?usp=drive_link. They were built this morning, so latest and greatest on the master branch. There you will also find my settings file (phoebus.ini).

As for snap_widgets, I cannot find any such string in properties files, only in Java code (DisplayEditor).

Regarding external apps configurations, I think they should be excluded from the cheks.

@lcaouen
Copy link
Contributor Author

lcaouen commented Dec 15, 2025

Thanks.
I have just downloaded the files.
Also I have just pushed a new version which should work with IntelliJ and exclude some keys from the check.

Now I'm gonna have a look to the snap_widgets and alarm/server keys

@lcaouen
Copy link
Contributor Author

lcaouen commented Dec 15, 2025

I have checked your settings.ini and the reason why the org.phoebus.applications.alarms/server is in red is because there is an error in your key name.
org.phoebus.applications.alarms/server sould be org.phoebus.applications.alarm/server without the s at alarm.
There is a second key like that : org.phoebus.logbook.ui/default_logbook_query sould be org.phoebus.logbook.olog.ui/default_logbook_query olog is missing in the key name

For the other keys in red, I cannot find them so I suppose it is some "secret" keys which are not in the properties files.

  • org.phoebus.logbook.olog.ui/show_log_entry_display_toolbar
  • org.phoebus.logbook.olog.ui/auto_properties
  • org.phoebus.applications.saveandrestore.logging/xxxx
  • org.phoebus.applications.saveandrestore.datamigration.git
  • org.phoebus.applications.saveandrestore/splitSaveset=false
  • org.phoebus.applications.saveandrestore/enableCSVIO=true
  • org.phoebus.applications.saveandrestore/splitSnapshot=false

Concerning the snap_widgets, I cannot reproduce it.
All the settings come from the settings stored in java.util.prefs.Preferences and loaded in org.phoebus.product.Launcher from the settings.ini file or PropertyPreferenceLoader (.phoebus in user directory). This is a part I haven't touched
I'm wondering if these settings could come from user.phoebus.userPrefs ?
Could you try to remove the folder and check if the key disapears or not ?
Maybe also check which user is used to launch Phoebus as the directory depends on the user.

@georgweiss
Copy link
Collaborator

Thanks @lcaouen for finding the errors I should have been able to spot myself.
As for the list of unknown preferences: those are obsolete preferences I have not removed from my ini file. Which proves your feature is quite useful :-)

The snap* stuff is indeed picked up from saved user prefs. Not sure if this can be handled and suppressed in your code.

Sadly I still see two issues:

  1. When running from a build I still see an empty preference listing. Reason I think is that in PropertyPreferenceWriter line 155 the File object is created from a relative file name. In such cases File.getParent() will return null.
  2. ESS adds a custom module with its own set of preferences, with its own namespace (eu.ess....). When I run such a configuration in IntelliJ, those preferences are highlighted in red.

@georgweiss
Copy link
Collaborator

@lcaouen, it would also be a good idea to replace all System.out.println() statements to something that logs information the way we usually do with a Logger instance.

@lcaouen
Copy link
Contributor Author

lcaouen commented Dec 16, 2025

No problem, I'm gonna use the standard logger and add a way to exclude custom packages and keys.

Could you tell me what you mean by "When running from a build" ? Which command line ? Which arguments and how do you give the settings.ini ?

@georgweiss
Copy link
Collaborator

Using the default way of building Phoebus into a tar ball or zip will create a single jar file (e.g. product-5.0.3-SNAPSHOT.jar) and then bundle all dependencies into the lib folder.
In your PR, PropertyPreferenceWriter.getAllJarFromManifest() I think tries to determine the file names of the dependencies listed in the ``product-5.0.3-SNAPSHOT.jarmanifest. This however fails as on line 155getParent()``` returns ```null``` since the file name of the ```File``` object is a relative file name.

@lcaouen
Copy link
Contributor Author

lcaouen commented Dec 17, 2025

Do you use phoebus.sh(.bat) in your phoebus-product to run it ?
Do you add any argument like -settings to give your ini file ?

image

When I run phoebus.bat without any argument (so no -settings given), I get this screen, which is correct as no settings file is loaded.
image

Is that what you get or something different ?

@georgweiss
Copy link
Collaborator

georgweiss commented Dec 18, 2025

No, we do not use the bundled shell script phoebus.sh. Instead we just launch Phoebus using something like:
java -jar <main jar file> -settings <settings file>

A few more JVM options are added, but that does not affect classpath.

@lcaouen
Copy link
Contributor Author

lcaouen commented Dec 19, 2025

@georgweiss I have pushed all the corrections.
Let me know if you see any other issue.
I will be off from this evening for 2 weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants